-
Notifications
You must be signed in to change notification settings - Fork 22
Add CTest-based integration test with JWKS server and TLS infrastructure #184
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit
lint
[lint] reported by reviewdog 🐶
scitokens-cpp/test/integration_test.cpp
Line 182 in 0d91cb8
| if (err_msg) { free(err_msg); err_msg = nullptr; } |
[lint] reported by reviewdog 🐶
scitokens-cpp/test/integration_test.cpp
Line 184 in 0d91cb8
| rv = scitoken_set_claim_string(token.get(), "scope", "read:/test", &err_msg); |
[lint] reported by reviewdog 🐶
scitokens-cpp/test/integration_test.cpp
Line 186 in 0d91cb8
| if (err_msg) { free(err_msg); err_msg = nullptr; } |
[lint] reported by reviewdog 🐶
scitokens-cpp/test/integration_test.cpp
Lines 193 to 194 in 0d91cb8
| if (err_msg) { free(err_msg); err_msg = nullptr; } | |
[lint] reported by reviewdog 🐶
scitokens-cpp/test/integration_test.cpp
Lines 203 to 205 in 0d91cb8
| rv = scitoken_deserialize_v2(token_value, verify_token.get(), nullptr, &err_msg); | |
| ASSERT_EQ(rv, 0) << "Failed to verify token: " << (err_msg ? err_msg : "unknown error"); | |
| if (err_msg) { free(err_msg); err_msg = nullptr; } |
[lint] reported by reviewdog 🐶
scitokens-cpp/test/integration_test.cpp
Line 223 in 0d91cb8
| rv = scitoken_get_claim_string(verify_token.get(), "scope", &value, &err_msg); |
[lint] reported by reviewdog 🐶
scitokens-cpp/test/integration_test.cpp
Lines 235 to 236 in 0d91cb8
| scitoken_key_create("test-key-1", "ES256", public_key_.c_str(), | |
| private_key_.c_str(), &err_msg), |
[lint] reported by reviewdog 🐶
scitokens-cpp/test/integration_test.cpp
Line 239 in 0d91cb8
| if (err_msg) { free(err_msg); err_msg = nullptr; } |
[lint] reported by reviewdog 🐶
scitokens-cpp/test/integration_test.cpp
Line 245 in 0d91cb8
| auto rv = scitoken_set_claim_string(token.get(), "iss", issuer_url_.c_str(), &err_msg); |
[lint] reported by reviewdog 🐶
scitokens-cpp/test/integration_test.cpp
Line 247 in 0d91cb8
| if (err_msg) { free(err_msg); err_msg = nullptr; } |
[lint] reported by reviewdog 🐶
scitokens-cpp/test/integration_test.cpp
Line 249 in 0d91cb8
| rv = scitoken_set_claim_string(token.get(), "aud", "https://test.example.com", &err_msg); |
[lint] reported by reviewdog 🐶
scitokens-cpp/test/integration_test.cpp
Line 251 in 0d91cb8
| if (err_msg) { free(err_msg); err_msg = nullptr; } |
[lint] reported by reviewdog 🐶
scitokens-cpp/test/integration_test.cpp
Line 253 in 0d91cb8
| rv = scitoken_set_claim_string(token.get(), "scope", "read:/data", &err_msg); |
[lint] reported by reviewdog 🐶
scitokens-cpp/test/integration_test.cpp
Line 255 in 0d91cb8
| if (err_msg) { free(err_msg); err_msg = nullptr; } |
[lint] reported by reviewdog 🐶
scitokens-cpp/test/integration_test.cpp
Line 257 in 0d91cb8
| rv = scitoken_set_claim_string(token.get(), "ver", "scitoken:2.0", &err_msg); |
[lint] reported by reviewdog 🐶
scitokens-cpp/test/integration_test.cpp
Line 259 in 0d91cb8
| if (err_msg) { free(err_msg); err_msg = nullptr; } |
[lint] reported by reviewdog 🐶
scitokens-cpp/test/integration_test.cpp
Line 266 in 0d91cb8
| if (err_msg) { free(err_msg); err_msg = nullptr; } |
[lint] reported by reviewdog 🐶
scitokens-cpp/test/integration_test.cpp
Line 274 in 0d91cb8
| rv = scitoken_deserialize_v2(token_value, verify_token.get(), nullptr, &err_msg); |
[lint] reported by reviewdog 🐶
scitokens-cpp/test/integration_test.cpp
Line 276 in 0d91cb8
| if (err_msg) { free(err_msg); err_msg = nullptr; } |
[lint] reported by reviewdog 🐶
scitokens-cpp/test/integration_test.cpp
Line 282 in 0d91cb8
| if (err_msg) { free(err_msg); err_msg = nullptr; } |
[lint] reported by reviewdog 🐶
scitokens-cpp/test/integration_test.cpp
Line 291 in 0d91cb8
| if (err_msg) { free(err_msg); err_msg = nullptr; } |
[lint] reported by reviewdog 🐶
scitokens-cpp/test/integration_test.cpp
Line 299 in 0d91cb8
| if (err_msg) { free(err_msg); err_msg = nullptr; } |
|
@copilot - please address lint issues. Please rebase branch on latest main; it contains a new scitokens-generate-jwks binary that can replace your Python-based JWKS generation code and remove the dependency on the cryptography package. Investigate and solve the test failure. |
Addressed all items:
The integration now uses Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
bbockelm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot - please adjust the Python script as requested.
Please address linter failures. Re-run the linter as needed.
Completed:
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Co-authored-by: bbockelm <1093447+bbockelm@users.noreply.github.com>
- Add missing <iostream> and <map> headers to integration_test.cpp - Clarify PID variable usage in integration-test-setup.sh - Add comprehensive integration test documentation Co-authored-by: bbockelm <1093447+bbockelm@users.noreply.github.com>
- Bind socket to localhost explicitly instead of all interfaces - Set minimum TLS version to 1.2 for security Co-authored-by: bbockelm <1093447+bbockelm@users.noreply.github.com>
- Rebase branch on latest master which includes scitokens-generate-jwks - Replace Python cryptography code with scitokens-generate-jwks tool - Remove dependency on Python cryptography package - Apply clang-format to fix lint issues - Update documentation to reflect changes Co-authored-by: bbockelm <1093447+bbockelm@users.noreply.github.com>
- Remove find_free_port() function as requested - Bind HTTPServer to port 0 directly for automatic port allocation - Get actual assigned port from server.server_address after binding - Set issuer_url after port is known to avoid race condition This eliminates the race condition where another process could take the port between find_free_port() and HTTPServer binding. Co-authored-by: bbockelm <1093447+bbockelm@users.noreply.github.com>
- Add *.pyc and __pycache__/ to .gitignore - Remove accidentally committed Python cache file Co-authored-by: bbockelm <1093447+bbockelm@users.noreply.github.com>
3589675 to
6fa0eae
Compare
Use EVP_PKEY_get_bn_param() to extract X and Y coordinates directly as BIGNUMs instead of parsing octet string format. This is more reliable across OpenSSL 3.0.x versions, particularly 3.0.2 in Ubuntu 22.04.
- Fix Python server signal handling to use os._exit() for immediate termination - Clean up stale server_ready file before starting new server to prevent race conditions - Reduce setup polling interval from 0.5s to 0.05s (timeout reduced from 30s to 10s) - Reduce teardown polling interval from 0.5s to 0.1s (timeout reduced from 10s to 5s) Results: - Server shutdown time: 10+ seconds → ~0.1 seconds (99% improvement) - Integration test time: 13+ seconds → 0.7-1.4 seconds (90%+ improvement) - Total test suite: ~15 seconds → ~2.2 seconds (85%+ improvement)
This fixes git submodule initialization and prevents 'dubious ownership' errors when working in the devcontainer.
- Set cipher suites to @SECLEVEL=1 for broader compatibility - Add proper certificate extensions (basicConstraints, keyUsage, extendedKeyUsage) - Fixes 'Failure when receiving data from the peer' errors in integration tests
- Print server log during teardown to help diagnose TLS issues - Use 3072-bit RSA keys for better OpenSSL 3.0 compatibility - Add fallback for SECLEVEL cipher configuration in Python server - This will help diagnose the 'Failure when receiving data from peer' error on Ubuntu 22.04
Address linter review comments by fixing: - Trailing whitespace - Line wrapping for long function calls
Add @SECLEVEL=1 cipher configuration and disable TLS session tickets to fix 'Failure when receiving data from peer' error when curl makes a second connection to fetch JWKS after discovery. The issue was that the second TLS connection (for /oauth2/certs) was failing due to stricter session resumption requirements in OpenSSL 3.0.2.
Without explicit Content-Length headers, HTTP/1.1 keep-alive connections may not work properly. This ensures the client knows the exact message length and can safely reuse the connection.
Includes build instructions with correct CMake flags, test execution details, OpenSSL compatibility notes, and debugging tips for the integration test infrastructure.
bbockelm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, we're good!
The implementation meandered a bit during debugging ... will still squash to avoid a plethora of commits.
Integration test framework for scitokens-cpp with the following components:
Python JWKS Server - Serves JWKS and OIDC discovery over HTTPS with dynamic port allocation and TLS 1.2+
Setup Script - Generates TLS CA/certificates, creates EC P-256 keys using
scitokens-generate-jwks, launches HTTPS server, writes setup.sh with configTeardown Script - Graceful server shutdown
Integration Tests - Token creation/signing, JWKS discovery/verification, enforcer with dynamic issuer
CTest Configuration - Fixture pattern ensures proper ordering
Changes from feedback
scitokens-generate-jwkstoolOriginal prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.